Quantize module to QQLinear - #3106
Conversation
angeloskath
left a comment
There was a problem hiding this comment.
This is very nice.
What I am thinking is whether we should have quantize_input as a necessary argument or not.
So far all arguments are mandatory. When mode was added that means that every module that needs to work with nn.quantize needs to accept a mode argument.
We could do the same for quantize_input or we could make it optional and nn.quantize only passes it if it is True.
For the quantize-able modules we have now If we don't intend to have all modules accept the parameter then ideally we should generalize the
|
|
Hadn't thought of I think passing it all the time makes sense. We did break the api for |
Good point. Do you think that it is needed? I had a feeling that typically embedding stays in
Did you have some other option in mind? |
I think that option is perfectly fine for now. The main question is if we intend to require modules handle the argument I maybe just slightly prefer the first version. So in that version The alternative would be to only pass I'm really okay with either way... we can always adapt later. It's quite a new feature so it's ok to be flexible based on how it's used. |
|
This is very nice! Minor request: would you mind adding a simple test in this file to make sure that quantizing a model with |
awni
left a comment
There was a problem hiding this comment.
Looks good to go other than missing test! I'll run the tests after you add a test and then we can merge when they clear.
Extended
Linear.to_quantize(..)andnn.quantize()to support quantization toQQLinear.Added a flag
quantize_inputthat defines the behaviour (default isFalse, soQuantizedLinearis used).